feat(ui-e2e): Playwright + Vite sandbox for cozystack-ui#12
feat(ui-e2e): Playwright + Vite sandbox for cozystack-ui#12myasnikovdaniil wants to merge 6 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces the ui-e2e skill to the Cozystack platform bundle, which automates the setup of a Playwright and Vite development environment for the cozystack-ui project. The feedback provided identifies several opportunities to improve the robustness of the skill's execution, including early verification of the kubectl binary, using safer git flags for worktree creation, and resolving a logical contradiction regarding port configuration and configuration file edits. Additionally, improvements were suggested for more reliable smoke testing and idiomatic use of the package manager.
| 2. `~/aenix/cozystack-ui` — the canonical clone on contributors' workstations. | ||
| 3. Otherwise — `AskUserQuestion` for the path. Do not `git clone`; the skill is for an already-cloned working copy. | ||
|
|
||
| Verify `pnpm` is on `$PATH` and the workspace's `package.json` has `"packageManager": "pnpm@..."`. If `pnpm` is missing, stop and tell the operator to install it — do not auto-install package managers. |
There was a problem hiding this comment.
The skill relies on kubectl in Phase 6, but only verifies pnpm in Phase 2. It is better to check for all required binaries early to avoid failures deep in the workflow.
| Verify `pnpm` is on `$PATH` and the workspace's `package.json` has `"packageManager": "pnpm@..."`. If `pnpm` is missing, stop and tell the operator to install it — do not auto-install package managers. | |
| Verify pnpm and kubectl are on $PATH and the workspace's package.json has "packageManager": "pnpm@...". If any are missing, stop and tell the operator to install them — do not auto-install tools. |
There was a problem hiding this comment.
Good catch — moved the binary check into Phase 2 so we fail fast instead of crashing five phases deeper when kubectl proxy is first invoked. Fixed in 3fbe074.
| Create a feature worktree off `main` for the upcoming fix work. The repo's CLAUDE.md is emphatic that the primary working directory must stay on its default branch — multiple Claude sessions share it. | ||
|
|
||
| ```bash | ||
| git -C <repo> worktree add <repo>/.claude/worktrees/<name> -b fix/<name> main |
There was a problem hiding this comment.
The git worktree add ... -b <branch> command will fail if the branch already exists (e.g., from a previous failed attempt where the worktree directory was removed but the branch remained). Using -B ensures the branch is reset to main for this new worktree, which is safer for a throwaway environment.
| git -C <repo> worktree add <repo>/.claude/worktrees/<name> -b fix/<name> main | |
| git -C <repo> worktree add <repo>/.claude/worktrees/<name> -B fix/<name> main |
There was a problem hiding this comment.
Keeping -b deliberately. -B would silently reset an existing fix/<name> branch and discard any commits the operator left on it from an earlier run. Phase 3 already prompts on worktree-path collisions; if the branch name collides too the operator gets an explicit git error they can recover from (delete the branch, pick a different --worktree=<name>, or reuse the existing one). The -B foot-gun isn't worth the convenience for a workflow that's meant to preserve the operator's in-progress work.
| Unless `--no-browser`: | ||
|
|
||
| ```bash | ||
| cd $WT/apps/console && npx playwright install chromium |
There was a problem hiding this comment.
There was a problem hiding this comment.
Agreed, switched to pnpm exec playwright install chromium — uses the workspace-installed binary directly and keeps the tooling story pnpm-only. Fixed in dca4314.
| Goal: a `kubectl proxy` instance is reachable on `$PROXY_PORT` and is talking to **the kubeconfig the operator picked**, not whatever happened to be active in the shell. | ||
|
|
||
| 1. Check whether `lsof -nP -iTCP:$PROXY_PORT -sTCP:LISTEN` (or `ss -tlnp`) already has a listener. | ||
| 2. If yes, inspect the existing process command line (`ps -p <pid> -o args=`). If it is a `kubectl proxy` pointing at the same kubeconfig (look for `--kubeconfig` in the args; if absent, check `KUBECONFIG` env in `/proc/<pid>/environ`), reuse it. Otherwise pick the next free port (start at `$PROXY_PORT+1`) and rewrite Vite's proxy `target` accordingly (see Phase 7). |
There was a problem hiding this comment.
There is a logical contradiction between this phase and Phase 7 (lines 126-129). This line suggests picking a new port and rewriting the Vite config, while Phase 7 states a preference for keeping the default port and avoiding edits to vite.config.ts. If a non-default port is chosen here, the application will fail to connect to the API unless the configuration is updated.
There was a problem hiding this comment.
Real contradiction — thanks for catching it. Phase 6 step 2 now stops and asks the operator (kill the occupant / pick a different proxy port / cancel) instead of silently switching the port and rewriting vite.config.ts. A port switch becomes an explicit operator decision that goes through Phase 7's vite.config.ts edit gate. Fixed in ed2f61d.
| CTX=$(kubectl --kubeconfig <path> config current-context) | ||
| kubectl --kubeconfig <path> --context $CTX proxy --port=$PROXY_PORT & | ||
| ``` | ||
| 4. Smoke-test with `curl -sS -o /dev/null -w "%{http_code}" http://localhost:$PROXY_PORT/api/v1/namespaces/default`. Expect `200`. On `401`/`403` the kubeconfig is wrong or expired — surface the error and stop. On connection refused, wait up to 5 s for the proxy to come up before failing. |
There was a problem hiding this comment.
Checking for the default namespace might fail due to RBAC restrictions in some clusters, even if the proxy is correctly established. Using the /api or /version endpoint is a more universal and reliable way to verify connectivity to the Kubernetes API.
| 4. Smoke-test with `curl -sS -o /dev/null -w "%{http_code}" http://localhost:$PROXY_PORT/api/v1/namespaces/default`. Expect `200`. On `401`/`403` the kubeconfig is wrong or expired — surface the error and stop. On connection refused, wait up to 5 s for the proxy to come up before failing. | |
| 4. Smoke-test with curl -sS -o /dev/null -w "%{http_code}" http://localhost:$PROXY_PORT/api. Expect 200. On 401/403 the kubeconfig is wrong or expired — surface the error and stop. On connection refused, wait up to 5 s for the proxy to come up before failing. |
There was a problem hiding this comment.
Good point — switched the smoke test to /api. It's the unauthenticated API-group discovery endpoint, so it answers 200 from any reachable apiserver and avoids false negatives on kubeconfigs that can't get namespaces/default due to RBAC. Fixed in c5b8a39.
| cd $WT && pnpm --filter @cozystack/console dev:e2e | ||
| ``` | ||
|
|
||
| Wait for the `ready in` / `Local:` line via a polling loop (`until grep -q ...`). Then `curl -sS -o /dev/null -w "%{http_code}" http://localhost:$PORT/` and expect `200`. |
There was a problem hiding this comment.
The grep command requires a source to check. Since the Vite server is started in the background, its output should be redirected to a log file (as mentioned in line 131) which can then be polled.
| Wait for the `ready in` / `Local:` line via a polling loop (`until grep -q ...`). Then `curl -sS -o /dev/null -w "%{http_code}" http://localhost:$PORT/` and expect `200`. | |
| Wait for the ready in / Local: line via a polling loop (until grep -q ... in the captured output file). |
There was a problem hiding this comment.
You're right, the background command had no output redirection — the grep -q loop had nothing to read. The dev server now writes to $WT/.cozystack-dev-ui-bootstrap-vite.log and the polling loop greps that file, with a 30 s timeout guard so a wedged Vite can't hang the skill forever. Fixed in 56cb00d.
0f12037 to
1ff8fe0
Compare
…zystack-ui Bootstraps a UI dev sandbox: brings up a feature worktree, installs @playwright/test + Chromium, writes playwright.config.ts, adds dev:e2e / test:e2e scripts, starts kubectl proxy against a chosen kubeconfig, and runs Vite on a free port. Lets an operator land in front of a running cozystack-ui pointed at any cluster without touching the repo's primary working tree. Named dev-ui-bootstrap (not ui-e2e) to follow the area-verb pattern used by sibling skills in the new scheme (talos-bootstrap, ubuntu-bootstrap, cluster-install, package-deploy, ...). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
364f1cc to
900771c
Compare
Address review feedback from gemini-code-assist on plugins/cozystack/skills/dev-ui-bootstrap/SKILL.md:36 — fail-fast on kubectl missing in Phase 2 instead of crashing deep in Phase 6 when `kubectl proxy` is first invoked. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Address review feedback from gemini-code-assist on plugins/cozystack/skills/dev-ui-bootstrap/SKILL.md:66 — the workspace is pnpm-managed; `pnpm exec` runs the workspace-installed binary directly without falling back to npx's lookup path. Keeps tooling consistent. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Address review feedback from gemini-code-assist on plugins/cozystack/skills/dev-ui-bootstrap/SKILL.md:114 — Phase 6 step 2 used to silently switch the proxy port and "rewrite Vite's proxy target accordingly", which contradicts Phase 7's preference for leaving vite.config.ts untouched. Phase 6 now stops and asks the operator when port 8001 is occupied by an unrelated process — kill / different port / cancel. A port switch becomes an explicit operator decision that goes through Phase 7's vite.config.ts edit gate. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
…es/default Address review feedback from gemini-code-assist on plugins/cozystack/skills/dev-ui-bootstrap/SKILL.md:120 — listing the default namespace can return 403 on locked-down kubeconfigs even when the proxy is up and the credentials are valid. /api is the unauthenticated API-group discovery endpoint and answers 200 from any reachable apiserver, so it is a more reliable proxy-reachability probe. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
… loop can read Address review feedback from gemini-code-assist on plugins/cozystack/skills/dev-ui-bootstrap/SKILL.md:137 — the background dev-server command had no output redirection, so the `until grep -q ...` polling loop had nothing to read. The server now writes to `$WT/.cozystack-dev-ui-bootstrap-vite.log` and the polling loop greps that file (with a 30 s timeout guard so a wedged Vite cannot hang the skill forever). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Summary
Adds
cozystack:ui-e2e— a skill that puts an operator in front of a runningcozystack-uiinstance with Playwright wired up, pointed at the cluster of their choosing.Stacked on top of #11 (uses the new
plugins/<plugin>/skills/<skill>/layout,cozystack:<name>invocation pattern, andtools/check-refs.shvalidation introduced there). Base branch isfeat/cozystack-install-skill; rebase / re-target tomainonce #11 merges.What the skill does
8 phases:
--kubeconfig,--worktree,--port,--proxy-port,--no-install,--no-browser).cozystack-uirepo (pwd →~/aenix/cozystack-ui→ ask)..claude/worktrees/offmain— never touches the primary working tree.pnpm install,pnpm --filter @cozystack/console add -D @playwright/test,npx playwright install chromium.apps/console/playwright.config.ts(no semicolons, baseURL with env-var override) and patchpackage.jsonscripts (dev:e2e,test:e2e).kubectl --kubeconfig <path> --context $CTX proxy --port=$PROXY_PORTand smoke-test against/api/v1/namespaces/default.Guardrails
AskUserQuestionif the picked context name containsprod.cozystack-uitree; never commits / pushes / opens PRs on the operator's behalf.cozystack-uiother than@playwright/test(the repo's CLAUDE.md is strict about bundle size).kubectl proxyon the port only if it's pointing at the same kubeconfig; otherwise picks the next free port.Registry changes
plugins/cozystack/.claude-plugin/plugin.json: version 1.14.1 → 1.15.0 (minor — new skill), description extended withui-e2e..claude-plugin/marketplace.json: cozystack plugin description extended withui-e2e.README.md: new row in thecozystackskills table; bundle layout updated to 11 skills.Validation
bash tools/check-refs.sh— all 5 checks green (references resolve, plugin:skill mentions resolve, descriptions list every skill, kubectl calls carry--kubeconfig, no private cluster names).cozystack-uiworktree before being lifted into this PR —pnpm dev:e2eboots on:3002,kubectl proxysmoke-tests200, Playwright runs against the SPA.Test plan
bash tools/check-refs.shpasses (CI gate from feat!(cozystack): wizard chain + 2-plugin consolidation (breaking) #11)./cozystack:ui-e2e --kubeconfig=~/.kube/<dev-cluster>end-to-end: worktree created, deps installed, both servers up, smoke checks green.playwright.config.tswithout prompt)./cozystack:ui-e2e --kubeconfig=<file-with-prod-context>triggers the prod re-confirm gate.🤖 Generated with Claude Code